Repository navigation
nginx: a missing certificate no longer stops the server - #14
Merged
Merged
Conversation
Without certificates/php-devforge.pem nginx refused to start and the container restarted forever, while Apache in the same state generated a self-signed certificate and served with a browser warning. Since task 38 made 'forge profile on nginx' a documented command, that is a state people will reach. nginx has no <IfFile>, so an entrypoint picks the certificate and hands it to the template as $SSL_CERT / $SSL_KEY. It also absorbed the envsubst calls inlined in CMD, which is now just openresty. Kept separate from Apache's copy on purpose: sharing twelve lines of openssl would mean widening both build contexts to ./docker-library. Planning turned up a second hole -- Apache's fallback had never been tested either, because CI always runs install_cert.sh first. The new step covers both, and checks the certificate's issuer rather than trusting 'curl -k', which accepts anything. Verified the step fails with the fix reverted, and that its trap puts the certificates back when it dies halfway.
The check grepped the issuer for 'CN=phpforge.dev'. The runner's OpenSSL prints 'CN = phpforge.dev' with spaces, this machine's does not, so it failed on GitHub after passing locally. It now compares issuer to subject. That is what self-signed means, and it does not depend on how a particular build formats a name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while verifying task 38 against a local stack. Without
certificates/php-devforge.pem— a fresh clone, or one whereinstall_cert.shwas skipped — nginx refuses to start and the container restarts forever:Apache in the same state generates a self-signed certificate and serves with a browser warning. Task 7 gave it that safety net; nginx never got one. Since #10 made
forge profile on nginxa documented command, this is a state people will reach.The fix
nginx has no equivalent of Apache's
<IfFile>, so the choice is made in an entrypoint and handed to the template as$SSL_CERT/$SSL_KEY. The templates were already rendered withenvsubst, so this adds two names to a variable list that exists precisely to keep nginx's own$mailand$subdomainsfrom being substituted.The entrypoint also absorbed the
envsubstcalls that were inlined inCMD, which is now just["openresty", "-g", "daemon off;"]— the same shape as the Apache image.Kept as a second copy rather than shared with Apache's: the common part is twelve lines of
openssl, and sharing it means widening both build contexts from./docker-library/{httpd,nginx}to./docker-library, shipping every other image's files as context.Planning turned up a second hole
Apache's fallback had never been tested either. Nothing in CI removed the certificates —
install_cert.shruns first, so one always existed. The net written in task 7 had been assumed to work for weeks. It does, verified here for the first time, but that is luck rather than coverage.The new step exercises both servers, and checks the certificate's issuer rather than trusting
curl -k, which accepts anything — an assertion built on it would pass even if no fallback were generated and a stale file were being served. A self-signed certificate has issuer equal to subject; mkcert's does not.Proven, not assumed
nginxdev is not runningtrap ... EXITnow restores them — verified by making it fail on purpose